-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add FXIOS-10094 [Unit Tests] Add store utility for tests #22172
Conversation
Generated by 🚫 Danger Swift against fc2e13a |
import Redux | ||
@testable import Client | ||
|
||
protocol StoreTestUtility { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In my opinion i think we might not need this protocol, i'd just add the StoreTestUtilityHelper
directly as property of the Tester class. Let me know what you think! thanks @cyndichin 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment! Hmm, I wanted a way to ensure that any test that uses the store will conform to the protocol, which will throw compiler errors if the user does not have the setup or reset methods and then can use the StoreTestUtilityHelper
for convenience. Happy to remove if you don't think it's useful though.
Does it seem unnecessary extra code for you? Do you think we should open up to the team?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok Thank you for clarifying @cyndichin yes that makes sense to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great, thanks! I'll also present this to the team tomorrow as well so can keep this PR open until then.
func resetTestingStore() | ||
} | ||
|
||
// Utility class used when replacing the global store for testing purposes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: i'd add a third /
so it is also visible for documentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated! will do this going forward and use ///
any cases where you'll use //
over ///
? Do you think this would be a good thing to standardize?
firefox-ios/firefox-ios-tests/Tests/ClientTests/Utils/StoreTestUtility.swift
Show resolved
Hide resolved
import Redux | ||
@testable import Client | ||
|
||
protocol StoreTestUtility { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah ok Thank you for clarifying @cyndichin yes that makes sense to me.
Presented during team meeting, seems good to merge. |
📜 Tickets
Jira ticket
Github issue
💡 Description
Create
StoreTestUtility
that can be conformed by test classes that want to override the global store with a testable store.Add
StoreTestUtilityHelper
which is a concrete class that can be used by the test class.📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120
)